Skip to content

test: cover FallbackChainService behavior - #43

Merged
AgentiLoop merged 8 commits into
AgentiLoop:mainfrom
harshitethic:test/fallback-chain-service
Sep 2, 2026
Merged

test: cover FallbackChainService behavior#43
AgentiLoop merged 8 commits into
AgentiLoop:mainfrom
harshitethic:test/fallback-chain-service

Conversation

@harshitethic

Copy link
Copy Markdown
Contributor

Summary

  • add coverage for FallbackChainService failure thresholds
  • cover fallback progression and disabled entries
  • cover success, reset, and clear state transitions
  • cover summary and FallbackEntry display behavior
  • isolate persisted UserDefaults state between tests

Closes #28

@AgentiLoop AgentiLoop left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ran the suite locally on macOS: xcodebuild test -only-testing:AgentTests/FallbackChainServiceTests → 10/10 passed (✔ Test run with 10 tests in 1 suite passed).

I verified every assertion against Agent/Services/FallbackChainService.swift and they're all correct reads of the state machine: maxFailuresBeforeFallback = 2 (first failure → nil, second → enabledChain[0] with consecutiveFailures reset), the enabled-only filtering in recordFailure/activeFallback/summary, recordSuccess/reset returning to primary, clear wiping chain + state, and the "→ " active marker in summary. The FallbackEntry.displayName raw-provider fallback test is right too. The test hygiene is genuinely good: @Suite(.serialized), and the withIsolatedService snapshot/restore of both the singleton state and the raw UserDefaults keys is careful (the raw-data restore at the end makes the re-added-entries-get-new-UUIDs detail harmless).

One requested change — out-of-scope deletions. The diff removes ~25 lines of explanatory comments from existing, unrelated tests: the allCriteriaDone gate rationale in GoalStateStoreTests, the entire doc comment on ToolFailureDetectionTests explaining the twice-broken isToolFailure history (including why the bare word "failed" was deliberately dropped as a trigger), and the polling-exemption rationale in StuckGuardFingerprintTests. Those comments document regression history — they're the reason future contributors won't reintroduce those exact bugs. Please revert every change above the // MARK: - FallbackChainService line so the diff is purely additive (the one whitespace fix at line 111 is fine either way).

Also: the PR body says Closes #28, and PR #40 claims the same issue. Since FallbackChainService genuinely had zero coverage, this PR is the better fit for #28 — I've asked #40 to drop its claim, so keep yours.

Everything else is merge-ready; happy to approve as soon as the comment deletions are reverted.

@AgentiLoop AgentiLoop closed this Aug 30, 2026
@AgentiLoop AgentiLoop reopened this Aug 30, 2026
@AgentiLoop AgentiLoop closed this Aug 30, 2026
@AgentiLoop AgentiLoop reopened this Aug 30, 2026
@AgentiLoop AgentiLoop closed this Aug 30, 2026
@AgentiLoop AgentiLoop reopened this Aug 30, 2026

@AgentiLoop AgentiLoop left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-checked after 3b924b0d (test: restore unrelated test comments):

  • Diff above // MARK: - FallbackChainService is now purely the blank-line whitespace fix at line 111 — the allCriteriaDone gate rationale in GoalStateStoreTests, the isToolFailure regression-history doc comment on ToolFailureDetectionTests, and the polling-exemption rationale in StuckGuardFingerprintTests are all back intact.
  • Closes #28 kept in the body — #40 has dropped its claim, so this PR is the one that closes the issue.
  • Ran locally: xcodebuild test -only-testing:AgentTests/FallbackChainServiceTests -only-testing:AgentTests/GoalStateStoreTests -only-testing:AgentTests/ToolFailureDetectionTests -only-testing:AgentTests/StuckGuardFingerprintTests → 36/36 passed in 4 suites.

Approving — diff is purely additive now.

@AgentiLoop
AgentiLoop merged commit c3a3a6b into AgentiLoop:main Sep 2, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add unit tests for an untested service in Agent/Services/

2 participants